[video_player] Move most Android event logic to Dart #10128

stuartmorgan-g · 2025-09-29T19:26:21Z

Rather than interpreting ExoPlayer event callbacks in Java and then sending event channel events that match the Dart platform interface, send the events directly and intepret them in Dart. This continues the process of reducing Java-side logic in the plugin.

In order to make this change less error-prone, it also adopts Pigeon event channels, so that the new events are type-safe; in order to do that, this switches to the Kotlin Pigeon generator, since Pigeon event channels haven't been implemented for Java.

Now that the buffer progress event synthesis is in Dart, this also fixes an existing TODO by reworking that logic to be based on an independent timer, rather than driven by playback position polling from the app-facing package. This both makes the logic clearer, and makes it work regardless of whether or not the video is paused.

Part of flutter/flutter#172763

Pre-Review Checklist

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

gemini-code-assist

Code Review

This pull request is a significant and well-executed refactoring that moves Android video event logic to Dart, improving type safety by adopting Pigeon event channels and switching to Kotlin for code generation. The changes simplify the native Android code by sending more raw events to be interpreted by the Dart side. Additionally, it improves the buffer progress update logic to be more reliable.

I've found two critical fall-through bugs in the new Dart event handling logic that would cause compilation errors, and a medium-severity issue related to the robustness of the new buffer polling timer. My review comments provide suggestions to fix these issues.

packages/video_player/video_player_android/lib/src/android_video_player.dart

gemini-code-assist · 2025-09-29T19:29:35Z

packages/video_player/video_player_android/lib/src/android_video_player.dart

+        _bufferPollingTimer = Timer.periodic(const Duration(seconds: 1), (
+          Timer timer,
+        ) async {
+          _updateBufferPosition(await _api.getBufferedPosition());
+        });


Using Timer.periodic with an async callback can lead to a few issues. If getBufferedPosition() takes longer than one second, another call will be initiated before the first one completes, causing overlapping executions. Additionally, if dispose() is called while getBufferedPosition() is in flight, _updateBufferPosition could be called after the player instance has been disposed.

A more robust pattern is to use a recursive Timer that schedules the next poll only after the previous one completes. For a minimal fix that addresses the use-after-dispose issue, I've added checks for _isDisposed before and after the await.

Suggested change

_bufferPollingTimer = Timer.periodic(const Duration(seconds: 1), (

Timer timer,

) async {

_updateBufferPosition(await _api.getBufferedPosition());

});

_bufferPollingTimer = Timer.periodic(const Duration(seconds: 1), (

Timer timer,

) async {

if (_isDisposed) {

timer.cancel();

return;

}

final int position = await _api.getBufferedPosition();

if (_isDisposed) {

return;

}

_updateBufferPosition(position);

});

If getBufferedPosition() takes longer than one second, another call will be initiated before the first one completes

I'm fine with potential bunching up of events if the main thread gets stuck for a second.

Additionally, if dispose() is called while getBufferedPosition() is in flight, _updateBufferPosition could be called after the player instance has been disposed.

Fixed.

stuartmorgan-g · 2025-09-29T19:27:17Z

...roid/src/main/java/io/flutter/plugins/videoplayer/texture/TextureExoPlayerEventListener.java

-      if (surfaceProducerHandlesCropAndRotation) {
-        // When the SurfaceTexture backend for Impeller is used, the preview should already
-        // be correctly rotated.
-        rotationCorrection = RotationDegrees.ROTATE_0;


This was a drive-by cleanup of a warning that the IDE linter (but not the command-line linter we have in CI) gave about setting this to the value it already had.

stuartmorgan-g · 2025-09-29T19:27:47Z

...roid/src/main/java/io/flutter/plugins/videoplayer/texture/TextureExoPlayerEventListener.java

    events.onInitialized(width, height, exoPlayer.getDuration(), rotationCorrection.getDegrees());
  }

-  private RotationDegrees getRotationCorrectionFromUnappliedRotation(


This was dead code missed when we dropped support for older API versions.

stuartmorgan-g · 2025-09-29T19:29:18Z

...eo_player_android/android/src/main/java/io/flutter/plugins/videoplayer/QueuingEventSink.java

-final class QueuingEventSink implements EventChannel.EventSink {
-  private EventChannel.EventSink delegate;
+final class QueuingEventSink {
+  private PigeonEventSink<PlatformVideoEvent> delegate;


I didn't bother to template the class since this is the only case we use. In the unlikely event that we need other types later, we can always adjust then.

stuartmorgan-g · 2025-09-29T19:29:53Z

...r/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java


  @Override
-  public void setLooping(@NonNull Boolean looping) {
+  public void setLooping(boolean looping) {


These minor type changes are because the Kotlin generator and Java generator don't produce identical interfaces.

stuartmorgan-g · 2025-09-29T19:30:29Z

...r/video_player_android/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayer.java

  }

  @Override
-  public @NonNull Messages.PlaybackState getPlaybackState() {


This multi-return getter was just to support the combined polling hack that is replaced by the timer.

stuartmorgan-g · 2025-09-29T19:31:26Z

...android/android/src/test/java/io/flutter/plugins/videoplayer/ExoPlayerEventListenerTest.java

-  }
-
-  @Test
-  public void onPlaybackStateChangedIdleAfterBufferingSendsBufferingEnd() {


There are fewer tests here now because the logic this was testing moved to Dart.

stuartmorgan-g · 2025-09-29T19:32:22Z

...roid/android/src/test/java/io/flutter/plugins/videoplayer/VideoPlayerEventCallbacksTest.java

-  }
-
-  @Test
-  public void onInitializedIncludesRotationCorrectIfNonZero() {


In the typed version, rotation is always sent, so there's no need to test with and without rotation differently.

stuartmorgan-g · 2025-09-29T19:33:59Z

packages/video_player/video_player_android/test/android_video_player_test.dart

-      const int playerId = 1;
-      const String mockChannel = 'flutter.io/videoPlayer/videoEvents$playerId';
-      TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
-          .setMockMessageHandler(mockChannel, (ByteData? message) async {


Rather than one giant test, which was hard to understand and maintain, I broke it apart into individual tests for each event/behavior being tested.

stuartmorgan-g added 6 commits September 29, 2025 14:56

Move all buffer update logic to a Dart-side timer

4baf6da

Remove unused function left over from API <21

e77fc16

Move onPlaybackStateChanged logic to Dart

4eacb22

Switch to Kotlin for Pigeon generation

146ff1d

Switch to Pigeon event channel

11ab600

Version bump

74112ca

stuartmorgan-g requested a review from ash2moon as a code owner September 29, 2025 19:26

github-actions bot added platform-android p: video_player labels Sep 29, 2025

gemini-code-assist bot reviewed Sep 29, 2025

View reviewed changes

stuartmorgan-g commented Sep 29, 2025

View reviewed changes

Avoid a potential send-event-after-disposed

4c52e9b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[video_player] Move most Android event logic to Dart #10128

[video_player] Move most Android event logic to Dart #10128

Uh oh!

stuartmorgan-g commented Sep 29, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

stuartmorgan-g Sep 29, 2025

Uh oh!

Uh oh!

[video_player] Move most Android event logic to Dart #10128

Are you sure you want to change the base?

[video_player] Move most Android event logic to Dart #10128

Uh oh!

Conversation

stuartmorgan-g commented Sep 29, 2025

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!